lib: micro-optimize EventEmitter#removeListener() #185
Conversation
|
👍 |
|
That's super sweet! Perhaps a pedantic point, but shouldn't we call it something else but |
Don't use Number#toPrecision(), it switches to scientific notation for numbers with more digits than the precision; use Number#toFixed(). PR-URL: nodejs#185 Reviewed-By: Chris Dickinson <[email protected]>
Replace the call to Array#splice() with a faster open-coded version that creates less garbage. Add a new benchmark to prove it. With the change applied, it scores a whopping 40% higher. PR-URL: nodejs#185 Reviewed-By: Chris Dickinson <[email protected]>
|
LGTM. I looked into splitting |
bnoordhuis
force-pushed
the
micro-optimize-ee-remove-listener
branch
2 times, most recently
from
1ee720b
to
d3f8db1
Compare
|
Cheers, fixed up the indentation and (partially) incorporated @algesten's feedback by renaming the function to spliceOne(). Landed in d3f8db1. @chrisdickinson Using Array#shift() to remove the first element is a bit faster, but only for larger lists. The lower bound is probably at least 16 elements but there doesn't seem to be an appreciable difference until the array length is in the low hundreds. |
Replace the call to Array#splice() with a faster open-coded version
that creates less garbage.
Add a new benchmark to prove it. With the change applied, it scores
a whopping 40% higher.
R=@chrisdickinson